expire azure token as needed #615
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| export async function createAzureToken( | ||
| signal: AbortSignal | ||
| ): Promise<AuthenticationToken> { |
There was a problem hiding this comment.
The parameter signal in the function createAzureToken is missing a type annotation. Please add a type annotation to improve code readability and maintainability. 📚
generated by pr-review-commit
missing_param_type
| if ( | ||
| !this._azureToken || | ||
| this._azureToken.expiresOnTimestamp >= Date.now() | ||
| ) |
There was a problem hiding this comment.
The comparison this._azureToken.expiresOnTimestamp >= Date.now() seems to be incorrect. It should be this._azureToken.expiresOnTimestamp <= Date.now() to check if the token has expired. 🕰️
generated by pr-review-commit
incorrect_comparison
| if ( | ||
| !this._azureToken || | ||
| this._azureToken.expiresOnTimestamp >= Date.now() | ||
| ) | ||
| this._azureToken = await createAzureToken(signal) |
There was a problem hiding this comment.
The function createAzureToken is an async function but it's not being awaited here. This could lead to unexpected behavior. Please use await to ensure the promise is resolved before proceeding. 🔄
generated by pr-review-commit
async_await
|
There are some potential functional issues in the pull request:
- this._azureToken.expiresOnTimestamp >= Date.now()
+ this._azureToken.expiresOnTimestamp <= Date.now()
- !this._azureToken ||
- this._azureToken.expiresOnTimestamp >= Date.now()
+ !this._azureToken ||
+ (typeof this._azureToken.expiresOnTimestamp === 'number' && this._azureToken.expiresOnTimestamp <= Date.now())These changes should make the code safer and functionally correct.
|
|
fix for #613 |
createAzureTokeninazuretoken.tshas been refactored to return an object of typeAuthenticationToken. This marks a shift from the previous version where it just returned a string token, to the new version where it returns an object containing two properties,token(a string) andexpiresOnTimestamp(a number). 🔄AuthenticationTokenwas introduced intoazuretoken.ts. 🆕_azureTokenprivate property innodehost.tshas been altered to hold anAuthenticationTokenobject as opposed to a single token string. 💾getLanguageModelConfigurationinnodehost.tshas been updated to refresh the_azureTokenif it is not set, or if itsexpiresOnTimestampis equal to or greater than the current time. This ensures that an expired token won't be used, improving reliability and security. ⏲️🔒getLanguageModelConfigurationnow uses_azureToken.tokenas opposed to_azureToken. 👍